Revert "[llvm][AArch64] Autoupgrade function attributes from Module attributes."#85291
Merged
DanielKristofKiss merged 1 commit intomainfrom Mar 14, 2024
Merged
Revert "[llvm][AArch64] Autoupgrade function attributes from Module attributes."#85291DanielKristofKiss merged 1 commit intomainfrom
DanielKristofKiss merged 1 commit intomainfrom
Conversation
…ttribute…" This reverts commit ded5de1.
Member
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-lto Author: Daniel Kiss (DanielKristofKiss) ChangesReverts llvm/llvm-project#82763 Full diff: https://github.com/llvm/llvm-project/pull/85291.diff 6 Files Affected:
diff --git a/llvm/include/llvm/IR/AutoUpgrade.h b/llvm/include/llvm/IR/AutoUpgrade.h
index 1ef32bcb121bec..152f781ffa9b30 100644
--- a/llvm/include/llvm/IR/AutoUpgrade.h
+++ b/llvm/include/llvm/IR/AutoUpgrade.h
@@ -88,9 +88,6 @@ namespace llvm {
/// info. Return true if module is modified.
bool UpgradeDebugInfo(Module &M);
- /// Copies module attributes to the functions in the module.
- void CopyModuleAttrToFunctions(Module &M);
-
/// Check whether a string looks like an old loop attachment tag.
inline bool mayBeOldLoopAttachmentTag(StringRef Name) {
return Name.starts_with("llvm.vectorizer.");
diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp
index 25992395e471b3..be0abb4b71dae2 100644
--- a/llvm/lib/IR/AutoUpgrade.cpp
+++ b/llvm/lib/IR/AutoUpgrade.cpp
@@ -5178,72 +5178,6 @@ void llvm::UpgradeFunctionAttributes(Function &F) {
Arg.removeAttrs(AttributeFuncs::typeIncompatible(Arg.getType()));
}
-// Check if the module attribute is present and not zero.
-static bool isModuleAttributeSet(Module &M, const StringRef &ModAttr) {
- const auto *Attr =
- mdconst::extract_or_null<ConstantInt>(M.getModuleFlag(ModAttr));
- return Attr && Attr->getZExtValue();
-}
-
-// Copy an attribute from module to the function if exists.
-// First value of the pair is used when the module attribute is not zero
-// the second otherwise.
-static void
-CopyModuleAttributeToFunction(Function &F, StringRef FnAttrName,
- StringRef ModAttrName,
- std::pair<StringRef, StringRef> Values) {
- if (F.hasFnAttribute(FnAttrName))
- return;
- F.addFnAttr(FnAttrName, isModuleAttributeSet(*F.getParent(), ModAttrName)
- ? Values.first
- : Values.second);
-}
-
-// Copy a boolean attribute from module to the function if exists.
-// Module attribute treated false if zero otherwise true.
-static void CopyModuleAttributeToFunction(Function &F, StringRef AttrName) {
- CopyModuleAttributeToFunction(
- F, AttrName, AttrName,
- std::make_pair<StringRef, StringRef>("true", "false"));
-}
-
-// Copy an attribute from module to the function if exists.
-// First value of the pair is used when the module attribute is not zero
-// the second otherwise.
-static void
-CopyModuleAttributeToFunction(Function &F, StringRef AttrName,
- std::pair<StringRef, StringRef> Values) {
- CopyModuleAttributeToFunction(F, AttrName, AttrName, Values);
-}
-
-void llvm::CopyModuleAttrToFunctions(Module &M) {
- Triple T(M.getTargetTriple());
- if (!T.isThumb() && !T.isARM() && !T.isAArch64())
- return;
-
- for (Function &F : M.getFunctionList()) {
- if (F.isDeclaration())
- continue;
-
- if (!F.hasFnAttribute("sign-return-address")) {
- StringRef SignType = "none";
- if (isModuleAttributeSet(M, "sign-return-address"))
- SignType = "non-leaf";
-
- if (isModuleAttributeSet(M, "sign-return-address-all"))
- SignType = "all";
-
- F.addFnAttr("sign-return-address", SignType);
- }
- CopyModuleAttributeToFunction(F, "branch-target-enforcement");
- CopyModuleAttributeToFunction(F, "branch-protection-pauth-lr");
- CopyModuleAttributeToFunction(F, "guarded-control-stack");
- CopyModuleAttributeToFunction(
- F, "sign-return-address-key",
- std::make_pair<StringRef, StringRef>("b_key", "a_key"));
- }
-}
-
static bool isOldLoopArgument(Metadata *MD) {
auto *T = dyn_cast_or_null<MDTuple>(MD);
if (!T)
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index a9afb6f7348a8f..37d21119447b9c 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1606,11 +1606,6 @@ Error IRLinker::run() {
// Loop over all of the linked values to compute type mappings.
computeTypeMapping();
- // Convert module level attributes to function level attributes because
- // after merging modules the attributes might change and would have different
- // effect on the functions as the original module would have.
- CopyModuleAttrToFunctions(*SrcM);
-
std::reverse(Worklist.begin(), Worklist.end());
while (!Worklist.empty()) {
GlobalValue *GV = Worklist.back();
diff --git a/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll b/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
index 74d9c86881d52c..ccf8cf67ede6dc 100644
--- a/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
+++ b/llvm/test/LTO/AArch64/link-branch-target-enforcement.ll
@@ -32,7 +32,6 @@ entry:
; CHECK-DUMP: <main>:
; CHECK-DUMP: bl 0x8 <main+0x8>
; CHECK-DUMP: <foo>:
-; CHECK-DUMP: paciasp
; `main` doesn't support BTI while `foo` does, so in the binary
; we should see only PAC which is supported by both.
diff --git a/llvm/test/LTO/AArch64/link-sign-return-address.ll b/llvm/test/LTO/AArch64/link-sign-return-address.ll
deleted file mode 100644
index c25857ceed7b40..00000000000000
--- a/llvm/test/LTO/AArch64/link-sign-return-address.ll
+++ /dev/null
@@ -1,43 +0,0 @@
-; Testcase to check that module with different branch-target-enforcement can
-; be mixed.
-;
-; RUN: llvm-as %s -o %t1.bc
-; RUN: llvm-as %p/Inputs/foo.ll -o %t2.bc
-; RUN: llvm-lto -exported-symbol main \
-; RUN: -exported-symbol foo \
-; RUN: -filetype=obj \
-; RUN: %t2.bc %t1.bc \
-; RUN: -o %t1.exe 2>&1
-; RUN: llvm-objdump -d %t1.exe | FileCheck --check-prefix=CHECK-DUMP %s
-; RUN: llvm-readelf -n %t1.exe | FileCheck --allow-empty --check-prefix=CHECK-PROP %s
-
-target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
-target triple = "aarch64-unknown-linux-gnu"
-
-declare i32 @foo();
-
-define i32 @main() {
-entry:
- %add = call i32 @foo()
- ret i32 %add
-}
-
-!llvm.module.flags = !{!0, !1, !2, !3 }
-!0 = !{i32 8, !"branch-target-enforcement", i32 0}
-!1 = !{i32 8, !"sign-return-address", i32 0}
-!2 = !{i32 8, !"sign-return-address-all", i32 0}
-!3 = !{i32 8, !"sign-return-address-with-bkey", i32 0}
-
-; CHECK-DUMP: <foo>:
-; CHECK-DUMP: paciasp
-; CHECK-DUMP: mov w0, #0x2a
-; CHECK-DUMP: autiasp
-; CHECK-DUMP: ret
-; CHECK-DUMP: <main>:
-; CHECK-DUMP-NOT: paciasp
-; CHECK-DUMP: str x30,
-; CHECK-DUMP: bl 0x14 <main+0x4>
-
-; `main` doesn't support PAC sign-return-address while `foo` does, so in the binary
-; we should not see anything.
-; CHECK-PROP-NOT: Properties: aarch64 feature: PAC
\ No newline at end of file
diff --git a/llvm/test/Linker/link-arm-and-thumb.ll b/llvm/test/Linker/link-arm-and-thumb.ll
index 37bd8c37f8b5e5..a90f2128e4430a 100644
--- a/llvm/test/Linker/link-arm-and-thumb.ll
+++ b/llvm/test/Linker/link-arm-and-thumb.ll
@@ -13,12 +13,11 @@ entry:
ret i32 %add
}
-; CHECK: define i32 @main() [[MAIN_ATTRS:#[0-9]+]]
+; CHECK: define i32 @main() {
; CHECK: define i32 @foo(i32 %a, i32 %b) [[ARM_ATTRS:#[0-9]+]]
; CHECK: define i32 @bar(i32 %a, i32 %b) [[THUMB_ATTRS:#[0-9]+]]
-; CHECK: attributes [[MAIN_ATTRS]] = { {{.*}} }
-; CHECK: attributes [[ARM_ATTRS]] = { {{.*}} "target-features"="-thumb-mode" }
-; CHECK: attributes [[THUMB_ATTRS]] = { {{.*}} "target-features"="+thumb-mode" }
+; CHECK: attributes [[ARM_ATTRS]] = { "target-features"="-thumb-mode" }
+; CHECK: attributes [[THUMB_ATTRS]] = { "target-features"="+thumb-mode" }
; STDERR-NOT: warning: Linking two modules of different target triples:
|
pranavk
approved these changes
Mar 14, 2024
nickdesaulniers
approved these changes
Mar 14, 2024
Member
nickdesaulniers
left a comment
There was a problem hiding this comment.
Thanks, this should unblock the performance regressions reported for LTO.
MaskRay
approved these changes
Mar 14, 2024
Member
MaskRay
left a comment
There was a problem hiding this comment.
Best to mention why it is reverted
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #82763 because caused a regressions with inlining.
See #84494 (comment)